-
Notifications
You must be signed in to change notification settings - Fork 4.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[JetChat] Add glance widget for JetChat App #1424 #1425
base: main
Are you sure you want to change the base?
Conversation
# Conflicts: # Jetchat/app/src/main/java/com/example/compose/jetchat/components/JetchatDrawer.kt
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your PR!
@@ -67,5 +67,8 @@ | |||
<string name="more_options">More options</string> | |||
<string name="touch_and_hold_to_record">Touch and hold to record</string> | |||
<string name="record_message">Record voice message</string> | |||
<string name="widget_loading_error">Cannot load Widget</string> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
<string name="widget_loading_error">Cannot load Widget</string> | |
<string name="widget_loading_error">Problem loading the widget</string> |
val appWidgetManager = AppWidgetManager.getInstance(context) | ||
val myProvider = ComponentName(context, WidgetReceiver::class.java) | ||
|
||
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of showing the error only when user clicks the button, can we prevent showing the drawer item at all?
Maybe you can abstract the method to something like widgetAddingIsSupported()
that uses the SDK check and uses the isRequestPinAppWidgetSupported
and check for that before showing the UI.
Jetchat/app/src/main/java/com/example/compose/jetchat/components/JetchatDrawer.kt
Show resolved
Hide resolved
@@ -64,6 +64,58 @@ private val initialMessages = listOf( | |||
"8:03 PM" | |||
) | |||
) | |||
val unreadMessages = listOf( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suggest instead of adding bunch of copy pastes, to use something like the initialMessages
and filter it for the unread messages. Maybe filter the ones not from the current author?
val unreadMessages = initialMessages.filter { it.author != "me" }
If you don't have enough messages, you can add more (meaningful-ish) messages to the initialMessages
<?xml version="1.0" encoding="utf-8"?> | ||
<RelativeLayout xmlns:android="http://schemas.android.com/apk/res/android" | ||
android:layout_width="match_parent" | ||
android:layout_height="match_parent" | ||
android:background="#ffffff" | ||
android:gravity="center"> | ||
|
||
<TextView | ||
android:layout_width="wrap_content" | ||
android:layout_height="wrap_content" | ||
android:textColor="@color/black30" | ||
android:text="@string/widget_loading_error" /> | ||
</RelativeLayout> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
where is this used? 🤔
android:background="#ffffff" | ||
android:gravity="center"> | ||
|
||
<TextView | ||
android:layout_width="wrap_content" | ||
android:layout_height="wrap_content" | ||
android:textColor="@color/black30" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Try not to use direct colors, but use the tokens from the Theme
style = TextStyle(fontWeight = FontWeight.Bold) | ||
) | ||
Text( | ||
text = message.content, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not a fan of us not being able to format the text in any form. Showing the unformatted font looks unstyled :(
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your contribution! I've added my feedback, and +1 all of @mlykotom's points.
Are you expecting to add live data to the widget too, or only stick with static data?
title = LocalContext.current.getString(R.string.app_name_unreads), | ||
) | ||
}) { | ||
val unreadMessage = unreadMessages |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
unreadMessages
should be hoisted and passed into MessagesWidget from provideGlance.
Usually in a Glance Widget you will pass to your widget a repository and the widget can access flows in the composable function. Even though this is fakeData we should try to keep as much as possible to this pattern.
Column(modifier = GlanceModifier.clickable(actionStartActivity<NavActivity>()).fillMaxWidth()) { | ||
Text( | ||
text = message.author, | ||
style = TextStyle(fontWeight = FontWeight.Bold) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
add color = GlanceTheme.colors.onSurface
to the TextStyle. This will make the colors update when you switch to dark mode.
) | ||
Text( | ||
text = message.content, | ||
style = TextStyle(fontWeight = FontWeight.Normal) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
add color = GlanceTheme.colors.onSurface to the TextStyle. This will make the colors update when you switch to dark mode.
import androidx.glance.appwidget.GlanceAppWidget | ||
import androidx.glance.appwidget.GlanceAppWidgetReceiver | ||
|
||
class WidgetReceiver : GlanceAppWidgetReceiver() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You have a widget error layout but do not use it. Consider referencing it here with the errorUiLayout property or implementing onCompositionError
Thanks for the review @secondsun and @mlykotom will work on these over the weekend and will re-request a review. |
Added an Unreads widget for the JetChat app using Glance. Also has a option to add widget to home screen on the app.
Glance.Widget.Recording.webm
Fixes #1424